Fix/444 input handle zero #445
Conversation
TerranceKhumalo-absa
requested review from
korel-san and
lukasmatta
as code owners
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a bug where the input component couldn't properly handle zero values by updating input and search handler methods to accept both string and number types. It also includes Angular dependency updates and import statement reorganization.
- Updated input handlers to accept
string | numbertypes to properly handle zero values - Upgraded Angular dependencies from 20.2.x to 20.3.x
- Reorganized imports for consistency
Reviewed Changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| cps-input.component.ts | Updated value handling to support both string and number types, preventing zero values from being treated as empty |
| cps-input.component.spec.ts | Added comprehensive test coverage for zero value handling and type flexibility |
| cps-datepicker.component.ts | Updated input value handler to accept string or number |
| cps-table.component.ts | Updated global filter method to accept string or number input |
| cps-tree-table.component.ts | Updated global filter method to accept string or number input |
| icons-page.component.ts | Updated search handler to accept string or number |
| colors-page.component.ts | Updated search handler to accept string or number |
| navigation-sidebar.component.ts | Updated search handler to accept string or number |
| package.json | Upgraded Angular dependencies to version 20.3.x |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| // eslint-disable-next-line @typescript-eslint/no-empty-function | ||
| onChange = (event: any) => {}; | ||
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
| onChange = (value: string | number) => {}; |
There was a problem hiding this comment.
The parameter name in the unused function comment should be 'value' instead of 'event' to match the updated signature. Consider updating the eslint-disable comment from @typescript-eslint/no-empty-function to @typescript-eslint/no-unused-vars since the parameter is now named, or remove the parameter name if it's not used.
There was a problem hiding this comment.
I know it's Friday, but you should not start drinking during office hours
|
I looked into this issue and I'm considering if extending the input value type is the right approach. With the current state, how can one assign a number type value (zero) for the input if the type of the input is defined as a string? Shouldn't type check during the compilation catch this (assuming |
…across components
This pull request primarily introduces improved type flexibility for input and search-related component methods, allowing them to accept both strings and numbers. Additionally, it updates Angular dependencies to the latest patch versions and makes minor import order adjustments for consistency and clarity.
Resolves #444
Type flexibility improvements:
CpsInputComponent,CpsDatepickerComponent,NavigationSidebarComponent,ColorsPageComponent,IconsPageComponent,CpsTableComponent, andCpsTreeTableComponentto accept bothstringandnumbertypes, converting values to strings where necessary to ensure consistent filtering and value handling. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10]Dependency updates:
package.jsonfrom20.2.xto20.3.xversions for improved stability and compatibility. [1] [2]Code consistency and import cleanup:
cps-datepicker.component.ts,cps-input.component.ts, andcps-table.component.ts. [1] [2] [3] [4]These changes enhance the robustness and maintainability of the codebase by supporting a wider range of input types and keeping dependencies up to date.
Release notes:
numbertype in input component and updated Angular to 20.3.x,